Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

avoid some more invalidations that are not necessary #49418

Merged
merged 1 commit into from
Apr 20, 2023

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented Apr 19, 2023

Even if we have a new method that is more specific than the method it is replacing, there still might exist an existing method that is more specific than both which already covers their intersection.

An example of this pattern is adding
Base.IteratorSize(::Type{<:NewType})
causing invalidations on
Base.IteratorSize(::Type)
for specializations such as
Base.IteratorSize(::Type{<:AbstractString})
even though the intersection of these is fully covered already by
Base.IteratorSize(::Type{Union{}})
so our new method would never be selected there.

This won't detect ambiguities that already cover this intersection, but that is why we are looking to move away from that pattern towards explicit methods for detection in closer to O(n) instead of O(n^2): #49349.

Similarly, for this method, we were unnecessarily dropping it from the MethodTable cache. This is not a significant latency problem (the cache is cheap to rebuild), but it is also easy to avoid in the first place.

Refs #49350

Even if we have a new method that is more specific than the method it is
replacing, there still might exist an existing method that is more
specific than both which already covers their intersection.

An example of this pattern is adding
    Base.IteratorSize(::Type{<:NewType})
causing invalidations on
    Base.IteratorSize(::Type)
for specializations such as
    Base.IteratorSize(::Type{<:AbstractString})
even though the intersection of these is fully covered already by
    Base.IteratorSize(::Type{Union{}})
so our new method would never be selected there.

This won't detect ambiguities that already cover this intersection, but
that is why we are looking to move away from that pattern towards
explicit methods for detection in closer to O(n) instead of O(n^2): #49349.

Similarly, for this method, we were unnecessarily dropping it from the
MethodTable cache. This is not a significant latency problem (the cache
is cheap to rebuild), but it is also easy to avoid in the first place.

Refs #49350
@vtjnash vtjnash merged commit b27c87e into master Apr 20, 2023
@vtjnash vtjnash deleted the jn/49350-definition-invalidations branch April 20, 2023 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant